Repository navigation
feat: allow cross projects zone access - #85
Conversation
0c132c0 to
19ab4a9
Compare
abhijith-darshan
changed the title
b6d64cd to
5d42cc8
Compare
5d42cc8 to
4c22d39
Compare
4c22d39 to
33e5f25
Compare
1f55541 to
1b7a3e2
Compare
frittentheke
left a comment
frittentheke
left a comment
There was a problem hiding this comment.
Thanks @abhijith-darshan for keeping at this and especially for going the extra mile to implement this in Gophercloud, the right place!
This chance looks good so far.
Since this s feature that needs integration testing, would you mind creating a devstack test for it? I know the current integration testing at https://github.com/inovex/external-dns-openstack-webhook/blob/main/.github/workflows/devstack.yml#L72-L109 is quite simple, but we already have devstack running ... it's just about marking use of it to ensure things work as they should with later changes.
3dce28f to
58cf282
Compare
I can try. Should this be a separate workflow or extend the existing one with a separate job or add additional step? |
|
@frittentheke how does this look? added webhook server and status server addr flags so that they can be configurable. Otherwise in the test we would have to kill existing running webhook. Also noticed the designate-worker fix was removed by you in the past - 3b52507 we just needed the abs path after [DEFAULT] and looks like the test time also reduces by 8 - 9 mins approx. |
|
Sorry about the delay @abhijith-darshan ! I shall try to look into this soon. |
Knalltuete5000
left a comment
Knalltuete5000
left a comment
There was a problem hiding this comment.
Minor details for the devstack but LGT
Done ... @frittentheke please review |
|
@frittentheke we now have an official release of gophercloud/v2.14.0 🎉 could you please review? We need to migrate our fleet from Ingress to Gateway API 😭 |
|
@frittentheke Could you please review? |
|
@abhijith-darshan I was on vacation and we should get this review done and merged this week. |
e8b7664 to
be42403
Compare
Done |
There was a problem hiding this comment.
Thanks @abhijith-darshan for all your patenience.
The PR looks good to me now as well!
I'd just like to ask your to squash the commits a little:
- The revert of the root helper fix (0ef0a71) should be a standalone commit (just blame me for removing it :-) )
- Your feature including all tests as a single commit
- The addition of configurable listeners in 8fcc57c
This reduces the number of commits from 13 to just 3.
If there are no changes to the syntax, I'll hit merge and cut a release.
designate-rootwrap lives in the devstack venv which is not in sudo's secure_path, causing rndc addzone to fail and zones to stay PENDING. Insert the absolute venv path as root_helper in designate.conf. Signed-off-by: abhijith-darshan <abhijith.ravindra@sap.com>
Add --all-projects flag to the webhook which sets X-Auth-All-Projects on every Designate API request, enabling an admin user to manage DNS records across projects it does not own. - ForEachZone, ForEachRecordSet, CreateRecordSet, UpdateRecordSet all pass AllProjects through to gophercloud opts - DeleteRecordSet uses recordsets.DeleteWithOpts (gophercloud v2.14.0) so the header is sent on deletes as well - Bump gophercloud/v2 to v2.14.0 which includes upstream PR #3844 - Add devstack CI test: zone owned by alt_demo project, webhook running as admin (not a member of alt_demo) creates records via --all-projects Signed-off-by: abhijith-darshan <abhijith.ravindra@sap.com>
Add --webhook-server-address (default 127.0.0.1:8888) and --status-server-address (default 0.0.0.0:8080) flags so operators can bind the servers to custom addresses without env vars. Signed-off-by: abhijith-darshan <abhijith.ravindra@sap.com>
5008582 to
04b03d0
Compare
3 participants
closes #83